Skip to content

Create new endpoint to save style editor schema per content type#36070

Open
dario-daza wants to merge 16 commits into
mainfrom
35781-create-new-endpoint-to-save-style-editor-schema-per-content-type
Open

Create new endpoint to save style editor schema per content type#36070
dario-daza wants to merge 16 commits into
mainfrom
35781-create-new-endpoint-to-save-style-editor-schema-per-content-type

Conversation

@dario-daza

@dario-daza dario-daza commented Jun 9, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Create the new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint to avoid editing other Content Type information when creating and saving a Style Editor Schema for a Content Type.

BE

  • Create the new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint to handle the metadata of the Content Types, specifically the DOT_STYLE_EDITOR_SCHEMA that is related to the Style Editor Schema.
  • Fix Postman tests using the new endpoint.
  • Create an integration test for the new endpoint.

FE

  • Call the new endpoint when saving Style Editor Schemas per Content Type.
  • Replace the use of DotCrudService with http.
  • Fix tests.

This PR fixes: #35781
This PR fixes: #35979

@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @dario-daza's task in 1m 33s —— View job


Rollback-Safety Analysis

  • Gather context from PR
  • Read rollback-unsafe categories reference
  • Get full PR diff
  • Analyze diff against all unsafe categories
  • Post findings and apply label

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / GraphQL / Headless API Contract Change
  • Risk Level: 🟡 MEDIUM
  • Why it's unsafe: This PR adds a new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint and updates the Angular frontend component in the same PR to call it — replacing the prior PUT /v1/contenttype/id/{idOrVar} call. If the backend is rolled back to N-1 while a browser or CDN still serves the N-version Angular bundle, any save of a Style Editor Schema will call the new endpoint (which does not exist on N-1), returning a 404. Users will be unable to save Style Editor Schemas until the browser/CDN cache clears.
  • Code that makes it unsafe:
    • dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java — new @PATCH @Path("/id/{idOrVar}/metadata") handler added (updateContentTypeMetadata, ~line 2033+)
    • core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/style-editor/dot-style-editor-builder.component.tsthis.#http.patch<DotCMSResponse>(\v1/contenttype/id/${contentType.id}/metadata`, metadataPatch)replaces the priorDotCrudService.putData` call to the full PUT endpoint (line ~230)
    • dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml — new path /v1/contenttype/id/{idOrVar}/metadata added
  • Alternative (if possible): The new endpoint is purely additive (a new path; no existing path removed or changed). The rollback impact is bounded: only the Style Editor Schema save fails with 404, and only while the N-frontend is cached. The old PUT /v1/contenttype/id/{idOrVar} is preserved, so operators can restore full functionality by clearing the CDN/browser cache after rollback. No database or Elasticsearch changes are involved.

No other unsafe categories matched:

  • No runonce tasks, DB schema changes, or data migrations (no C-1, C-2, C-3, C-4, H-1 through H-7)
  • No OSGi interface changes (no M-4)
  • No push-publishing bundle format changes (no M-2)
  • The WorkflowResource.java change is a trivial .collect(Collectors.toList()).toList() stream refactor — not a contract change

@dario-daza dario-daza marked this pull request as ready for review June 9, 2026 14:57
@dario-daza dario-daza changed the title 35781 create new endpoint to save style editor schema per content type Create new endpoint to save style editor schema per content type Jun 9, 2026
@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / GraphQL / Headless API Contract Change
  • Risk Level: 🟡 MEDIUM
  • Why it's unsafe: This PR adds a new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint and updates the Angular frontend component in the same PR to call it (replacing the previous PUT /v1/contenttype/id/{idOrVar} call). If the backend is rolled back to N-1 while a browser or CDN has cached the N-version Angular frontend, any attempt to save a Style Editor Schema will call the new endpoint which does not exist on N-1, resulting in a 404 error. The user will be unable to save Style Editor Schemas until the browser cache clears.
  • Code that makes it unsafe:
    • dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java — new @PATCH @Path("/id/{idOrVar}/metadata") handler added at line 2022+
    • core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/style-editor/dot-style-editor-builder.component.ts — line 247: this.#http.patch\<DotCMSResponse\>(v1/contenttype/id/${contentType.id}/metadata, metadataPatch) replaces the prior DotCrudService.putData call to the old PUT endpoint
    • dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml — new path /v1/contenttype/id/{idOrVar}/metadata added (lines 8148+)
  • Alternative (if possible): Since the new endpoint is purely additive (a new path, no existing path removed or changed), the rollback risk is bounded: only the Style Editor Schema save fails with 404, and only while the N-frontend is cached. If the old PUT /v1/contenttype/id/{idOrVar} endpoint is preserved (which it is — this PR does not remove it), operators can restore full functionality by clearing the CDN cache and/or forcing a frontend refresh after rollback.

Comment thread dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java Outdated
…ed lock

Moves the read-merge-save logic and DOT_STYLE_EDITOR_SCHEMA normalization
out of ContentTypeResource into ContentTypeHelper#mergeAndSaveMetadata.
The operation is now serialized per Content Type using a JVM-local striped
lock (DotConcurrentFactory.getIdentifierStripedLock), re-reading inside the
lock to prevent lost updates from concurrent PATCH requests on the same
Content Type. Adds a concurrency integration test that asserts both keys
survive when two threads patch simultaneously.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / GraphQL / Headless API Contract Change
  • Risk Level: 🟡 MEDIUM
  • Why it's unsafe: This PR adds a new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint and updates the Angular frontend component in the same PR to call it (replacing the previous PUT /v1/contenttype/id/{idOrVar} call). If the backend is rolled back to N-1 while a browser or CDN has cached the N-version Angular frontend, any attempt to save a Style Editor Schema will call the new endpoint which does not exist on N-1, resulting in a 404 error. The user will be unable to save Style Editor Schemas until the browser cache clears.
  • Code that makes it unsafe:
    • dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java — new @PATCH @Path("/id/{idOrVar}/metadata") handler added
    • core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/style-editor/dot-style-editor-builder.component.tsthis.#http.patch<DotCMSResponse>(v1/contenttype/id/${contentType.id}/metadata, metadataPatch) replaces the prior DotCrudService.putData call to the old PUT endpoint
    • dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml — new path /v1/contenttype/id/{idOrVar}/metadata added
  • Alternative (if possible): Since the new endpoint is purely additive (a new path, no existing path removed or changed), the rollback risk is bounded: only the Style Editor Schema save fails with 404, and only while the N-frontend is cached. The old PUT /v1/contenttype/id/{idOrVar} endpoint is preserved (not removed by this PR), so operators can restore full functionality by clearing the CDN cache and/or forcing a frontend refresh after rollback.

@github-actions

Copy link
Copy Markdown
Contributor

Security Review

No high-confidence security findings on the changes in this PR.

@fabrizzio-dotCMS

Copy link
Copy Markdown
Member

Question: is metadata preserved when a Content Type is saved through the regular full PUT?

This new endpoint (PATCH /api/v1/contenttype/id/{idOrVar}/metadata) merges into the existing metadata and serializes DOT_STYLE_EDITOR_SCHEMA correctly — that part looks good.

My concern is the interaction with the regular update path (putContentTypeUpdatePUT /api/v1/contenttype/id/{idOrVar}), whose Swagger description states:

⚠️ Destructive semantics. This endpoint treats the request body as the full desired state. Any editable property (including items in fields[] and metadata keys) absent from the body will be removed.

So if any client does a normal full-CT PUT without carrying DOT_STYLE_EDITOR_SCHEMA in metadata, the schema written by this new endpoint would be silently wiped.

  1. Does the CT edit screen (or any existing client) round-trip the full metadata map — including DOT_STYLE_EDITOR_SCHEMA — on a regular save? If not, a normal save would clobber the style editor schema.
  2. Should the style editor schema be protected against the full-PUT's destructive merge (e.g. preserved server-side unless explicitly cleared), or is the contract strictly "GET → mutate → PUT the whole object"?

Mainly want to make sure the schema persisted here survives a subsequent normal CT save.

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

❌ Codex Review failed — openai.gpt-5.5

The review job failed before producing output. See the run for details.

Run: #27579601571

@dario-daza

Copy link
Copy Markdown
Member Author

Question: is metadata preserved when a Content Type is saved through the regular full PUT?

This new endpoint (PATCH /api/v1/contenttype/id/{idOrVar}/metadata) merges into the existing metadata and serializes DOT_STYLE_EDITOR_SCHEMA correctly — that part looks good.

My concern is the interaction with the regular update path (putContentTypeUpdatePUT /api/v1/contenttype/id/{idOrVar}), whose Swagger description states:

⚠️ Destructive semantics. This endpoint treats the request body as the full desired state. Any editable property (including items in fields[] and metadata keys) absent from the body will be removed.

So if any client does a normal full-CT PUT without carrying DOT_STYLE_EDITOR_SCHEMA in metadata, the schema written by this new endpoint would be silently wiped.

  1. Does the CT edit screen (or any existing client) round-trip the full metadata map — including DOT_STYLE_EDITOR_SCHEMA — on a regular save? If not, a normal save would clobber the style editor schema.
  2. Should the style editor schema be protected against the full-PUT's destructive merge (e.g. preserved server-side unless explicitly cleared), or is the contract strictly "GET → mutate → PUT the whole object"?

Mainly want to make sure the schema persisted here survives a subsequent normal CT save.

The way we handled it in the frontend, it never happens, but it is possible that any REST API caller (Postman, a migration script, or a third-party integration) that makes a raw PUT and omits metadata from the body would silently wipe the schema. I added a validation that prevents this and preserves the metadata if a user omits it in a PUT request. If you want to wipe the metadata, you'll need to send metadata: null.

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — deepseek.v3.2

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1026 — Missing @WrapInTransaction annotation on mergeAndSaveMetadata method. This method performs a read-modify-write sequence (find, save) and should be transactional to ensure data integrity.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1030 — The lock key "ct-metadata-" + initial.id() is not guaranteed to be unique across different entity types, which could cause unnecessary contention. Consider a more specific prefix like "contenttype-metadata-".

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1052 — The catch block catching Throwable is too broad and could swallow unexpected errors like OutOfMemoryError. It should catch specific exceptions (DotDataException, DotSecurityException, RuntimeException) and rethrow others.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1085 — The normalizeStyleEditorSchemaToString method logs a warning but then throws a BadRequestException. The warning is redundant with the exception and could clutter logs.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1109 — The requestContainsKey method logs a warning on parsing failure but returns false. This silent failure could mask malformed JSON in requests; consider throwing BadRequestException instead.

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java:792 — The logic to preserve metadata when the key is absent relies on contentType.metadata() == null. If the incoming contentType already has a metadata map (even empty), it will not preserve the existing metadata, which could be a bug.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java:1105 — The method returns HTTP 200 OK with an empty patch, but logs a warning. This is inconsistent; a no-op should likely be silent (debug log) or return a different status (e.g., 204 No Content).

[🟡 Medium] dotcms-integration/src/test/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResourceUpdateMetadataTest.java:430 — The test file is missing a final newline, which violates common code style conventions.

[🟡 Medium] dotcms-postman/src/main/resources/postman/Define_Contentlets_StyleProperties.postman_collection.json — The Postman collection updates are extensive but appear to be functional changes to align with the new PATCH endpoint. No bugs or security issues are evident in the diff snippets provided.


Run: #27624354227 · tokens: in: 19293 · out: 663 · total: 19956

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Not Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

3 participants